Investigate and reproduce NoMethodError: undefined method 'safe_concat' for nil on splash page with YJIT enabled#1
Open
ibrahima wants to merge 1 commit into
Conversation
|
🔗 This pull request is linked to Superconductor implementation. |
There was a problem hiding this comment.
Pull request overview
This PR addresses a YJIT register-mapping bug for methods with very large local tables by preventing oversized local indices from wrapping into tracked local register slots.
Changes:
- Replaces a truncating
as u8cast inOpnd::get_reg_opndwith a checked conversion that maps out-of-range locals to an untracked sentinel. - Adds a bootstrap regression test intended to reproduce the large-local Slim-style method failure.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
yjit/src/backend/ir.rs |
Updates local operand register mapping to avoid u8 wraparound for high local indices. |
bootstraptest/test_yjit.rb |
Adds a regression scenario with 256+ generated locals and repeated rendering calls. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| # regression test for register mapping of methods with over 256 locals | ||
| assert_normal_exit %q{ |
superconductor-for-github
Bot
force-pushed
the
ibrahima/3-investigate-yjit-splash-page-error/6
branch
6 times, most recently
from
42fb311 to
e152e79
Compare
Previously, local indices greater than 255 were truncated when converted to RegOpnd::Local. This could make a high-index local alias a tracked low-index local in the register mapping and produce incorrect values after compilation. Cap overflowing indices to u8::MAX. Since this is greater than MAX_CTX_LOCALS, YJIT treats these locals as untracked. Fixes [Bug #22074]. Co-authored-by: Codex <199175422+chatgpt-connector[bot]@users.noreply.github.com>
superconductor-for-github
Bot
force-pushed
the
ibrahima/3-investigate-yjit-splash-page-error/6
branch
from
e152e79 to
0f35c62
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation / Background
Fixes a YJIT miscompilation bug where methods with more than 256 local variables caused integer truncation when mapping locals to registers. In
Opnd::get_reg_opnd, a local index of 256 was cast tou8, wrapping to0, causing YJIT to treat an untracked high-index local as tracked local0. This led to register aliasing, memory corruption, andNoMethodError: undefined method 'safe_concat' for nilcrashes on the Superconductor splash page when YJIT was enabled.The root cause was traced to Slim's code generation: the old splash partial compiled into a single enormous method with enough locals (>256) to trigger the wrap. The bug reproduces on both Ruby 3.4.x and 4.0.x with YJIT enabled and is distinct from the previously fixed Ruby bug #21941.
Detail
This Pull Request changes:
yjit/src/backend/ir.rs: Replaces the truncatingas u8cast inOpnd::get_reg_opndwith a checked conversion (try_into().unwrap_or(u8::MAX)). Local indices that don't fit inu8now resolve tou8::MAX(255), whichRegMapping::alloc_regcorrectly rejects as untrackable (sinceMAX_CTX_LOCALSis 8). This prevents oversized indices from wrapping into the trackable0..7range.bootstraptest/test_yjit.rb: Adds a regression test that generates a Slim-shaped method with 128 repeated attribute blocks (~260+ locals), then calls it 40 times to trigger YJIT compilation. Before the fix, this fails around iteration 30 withNoMethodError; after the fix it completes cleanly.Additional information
>= MAX_CTX_LOCALS). The bug was accidentally granting unsound register allocation to wrapped indices, not a valid optimization being removed.u8::MAX(255) is safe as a sentinel becauseRegMapping::alloc_reggates onlocal_idx >= MAX_CTX_LOCALS as u8(i.e.,>= 8), so 255 is firmly rejected.Checklist
Superconductor Ticket Implementation | App Preview | Guided Review